Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose PopupProperties in PopupMenu #531

Merged
merged 2 commits into from
Aug 8, 2024

Conversation

francisconoriega
Copy link
Contributor

PopupMenu internally uses Popup, but uses a hardcoded PopupPropterties object. This exposes it as a param and uses the old hard coded values as the default parameter value.

@rock3r rock3r self-requested a review August 8, 2024 17:20
@rock3r rock3r added the api Changes related to the public API label Aug 8, 2024
@rock3r rock3r enabled auto-merge (squash) August 8, 2024 17:21
@rock3r
Copy link
Collaborator

rock3r commented Aug 8, 2024

@francisconoriega you need to run apiDump :)

auto-merge was automatically disabled August 8, 2024 17:42

Head branch was pushed to by a user without write access

@francisconoriega
Copy link
Contributor Author

@francisconoriega you need to run apiDump :)

I thought it was an automatic CI step :P.

Would it make sense to run the klint, api-dump, and other tasks as as pre-commit hooks?

@rock3r
Copy link
Collaborator

rock3r commented Aug 8, 2024

@francisconoriega it's not automatic because we want to ensure people are aware they're changing the APIs and do it knowingly. When we reach 1.0 we won't allow breaking changes anymore, unless it's a major change.

Would it make sense to run the klint, api-dump, and other tasks as as pre-commit hooks?

I have a pre-push hook locally that runs ktlintCheck, we should eventually add it as a built-in. I looked into ways to do that "properly" in the past but never settled on one solution. Do you know any Gradle plugin to manage and install hooks?

I don't love the idea of running it as pre-commit as it's very slow. Plus, since we squash, we only care about the formatting when we push.

@rock3r rock3r merged commit dca6945 into JetBrains:main Aug 8, 2024
3 checks passed
@francisconoriega francisconoriega deleted the fn-popup-menu-properties branch August 8, 2024 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Changes related to the public API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants